Add inlineDataParts accessor for GenerateContentResponse #14755
Conversation
Adds a new computed property `inlineDataParts` to `GenerateContentResponse` to provide easier access to inline data parts (like images) within the model's response. This property iterates through the parts of the first candidate and filters for `InlineDataPart` instances. Error handling is included for cases with no candidates or no inline data parts. Unit tests have been added in `GenerativeModelTests.swift` to validate the functionality of the new property, covering scenarios with and without inline data, and responses with no candidates.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback. |
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request adds an inlineDataParts accessor to the GenerateContentResponse struct, mirroring implementations in Android and JS. The code includes appropriate logging and unit tests, which is great. Here are a few suggestions for improvement.
Summary of Findings
- Clarity of Log Message: The log message could be more specific about the context in which the inline data parts are being accessed. Consider adding information about which candidate is expected to contain the inline data.
- Test Coverage: While the unit tests cover cases with and without inline data, consider adding a test case where multiple candidates exist, but only the first one is checked for inline data parts. This would more closely reflect the intended behavior of the accessor.
- Naming Consistency: Consider renaming
inlineDatatoinlineDataPartsfor consistency with the accessor nameinlineDataParts.
Merge Readiness
The code changes seem to be in good shape, but there are a few suggestions for improvement. I would recommend addressing these before merging. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging.
Generated by 🚫 Danger |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Andrew Heard <andrewheard@google.com>
Co-authored-by: Andrew Heard <andrewheard@google.com>
This should match the Android and JS implementations